Generic UART driver based on Device Trees#72
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a device-tree-driven, generic UART stack (codegen + HAL + kernel driver + uAPI) for STM32L4, and switches the kernel console UART selection from an environment variable to chosen.osiris,console in the DTS. It also integrates UART interrupts with the dynamic IRQ registration system.
Changes:
- Extend
dtgento emit a UART registry/query API from DT pinctrl + interrupts, and resolve the console UART fromchosen.osiris,console. - Add a kernel UART driver (blocking + NB + waiter/callback wakeups) and expose it via
src/uapi::uart. - Replace the previous “debug UART via OSIRIS_DEBUG_UART” path with a DT-driven console UART, updating Cortex-M init/print and presets accordingly.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| xtasks/crates/dtgen/src/codegen.rs | Adds UART DT registry/query codegen, shared STM32 pinmux decode helper, console resolution via chosen.osiris,console. |
| src/uapi/uart.rs | Adds uAPI re-export and open() wrapper for UART devices. |
| src/uapi.rs | Exposes the new uapi::uart module. |
| src/sched.rs | Adds kick_by_uid and exports an ISR-safe kick_thread C-ABI symbol. |
| src/drivers/uart/wait.rs | Implements IRQ-driven waiter lists/callback dispatch and dynamic IRQ registration for UART slots. |
| src/drivers/uart/mod.rs | Adds UART Device abstraction with blocking/NB read/write and timeout handling. |
| src/drivers.rs | Registers the UART driver module in the kernel drivers tree. |
| presets/stm32l4r5zi_def.toml | Removes OSIRIS_DEBUG_UART preset setting (console now selected via DTS). |
| machine/cortex-m/st/stm32l4/interface/uart.h | Introduces new STM32L4 UART interface API (IT-driven rings + console mode). |
| machine/cortex-m/st/stm32l4/interface/uart.c | Implements multi-UART slot management, ring buffers, and HAL callbacks for STM32L4. |
| machine/cortex-m/st/stm32l4/interface/export.h | Switches exported UART interface to uart.h instead of debug-uart-only symbols. |
| machine/cortex-m/src/stub/uart.rs | Adds stub UART HAL for non-hardware builds. |
| machine/cortex-m/src/stub.rs | Exposes the new stub UART module. |
| machine/cortex-m/src/native/uart.rs | Adds native UART HAL glue: DT registry lookup, C FFI calls, console init/write. |
| machine/cortex-m/src/native.rs | Initializes console UART from DT and routes Machine::print through it. |
| machine/cortex-m/build.rs | Errors out if OSIRIS_DEBUG_UART is set; continues DT generation and vector table generation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| } | ||
|
|
||
| fn duration_to_ticks(d: Duration) -> u64 { |
There was a problem hiding this comment.
we should maybe put that directly in the time module
# Conflicts: # machine/cortex-m/src/native.rs # machine/cortex-m/src/stub.rs # src/sched.rs # xtasks/crates/dtgen/src/codegen.rs
RX was one interrupt + one wake per byte (HAL_UART_Receive_IT(...,1) re-armed in HAL_UART_RxCpltCallback) — ~11.5k IRQ/s at 115200. Use the STM32L4 hardware FIFO instead: - uart_hw_init (non-console only): set TX/RX FIFO thresholds then HAL_UARTEx_EnableFifoMode, after HAL_UART_Init. - uart_init_common: arm HAL_UARTEx_ReceiveToIdle_IT into a 32-byte per-slot scratch buffer. The hardware IDLE line flushes whatever has arrived ~1 char-time after the last byte, so latency stays low while bursts coalesce into one callback/wake instead of one per byte. - HAL_UART_RxCpltCallback -> HAL_UARTEx_RxEventCallback: copy Size bytes scratch->rx_ring (unchanged drop-on-full policy), wake once, re-arm. TX path unchanged; FIFO + threshold batch it internally. - Add HAL_UART_ErrorCallback: ORE/overrun aborts ReceiveToIdle with no RxEvent, so re-arm there to self-heal RX. - Move slot->in_use=1 to the last write in uart_init_common so a failed open no longer leaves a slot stuck in-use. Console (blocking, no IT) untouched. Public ABI unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…link) The FIFO + HAL_UARTEx_ReceiveToIdle_IT rewrite broke the IT UART link on hardware (no TX/RX). Restore the proven per-byte HAL_UART_Receive_IT implementation from before the FIFO change. The FIFO work remains in history for a future hardware-validated attempt. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Restores the FIFO + HAL_UARTEx_ReceiveToIdle_IT driver. RX FIFO threshold is 1/8 so the FIFO ISR drains every byte (RXFT re-fires while RXFNE) and the HAL's IDLE handler never strands sub-threshold bytes — the per-frame RxEventCallback still cuts scheduler wakes and the 8-byte FIFO cushions ISR latency. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collapse src/drivers/uart/{mod.rs,wait.rs} into one src/drivers/uart.rs,
matching the single-file layout of the other drivers (e.g. can.rs).
Layout: public types + global state + Device at the top, free-function
helpers (conversions, waiter registration, ISR dispatch) at the bottom.
Replace the bespoke per-slot intrusive waiter list (Waiter with
Cell<*mut Waiter>, unsafe impl Sync, IRQ-disable pointer splicing, a
transmute'd callback pointer — 205 lines, 10 unsafe sites) with two
ParkedWaiter per slot (RX/TX), the kernel's existing single-consumer
park/wake primitive that can.rs already uses. The merged file has 2
unsafe sites, both unavoidable and identical in discipline to can.rs:
the FFI crate::irq::register_irq call and the single &*(ctx as *const _)
cast in the ISR dispatch.
Drop the dead ChannelCallback / set_*_callback API and the unused
public register/unregister_*_waiter pass-throughs (no callers).
deinit() failures (open cleanup path and Drop) now warn! instead of
being silently discarded.
Behavior changes are confined to previously-unused paths:
- multi-waiter per channel removed (only ever 1 RX + 1 TX per slot);
- a 2nd concurrent reader/writer on a slot now returns Error::Busy
(ParkedWaiter EBUSY) instead of silently queuing — matches the
one-consumer-per-controller invariant the CAN driver documents.
read/write_with_timeout keep their exact timeout semantics. The
IPSR-indexed irqn+16, vector_dispatch, and ensure_registered are
unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Did some bidirectional communication testing with an UART to TTL converter between Linux and an Nucleo-L4R5 board, and it generally seems to hold up well |
- Do not keep registered state if things error early - Re-add timeout for console logs - Fix stuck tx like rx
Imagine #41, except that you can now define UART pins in the device tree definition.
Also uses the new dynamic IRQ system.
Mostly tested via debug UART.